Serializer: Fixes unsafe stream cast in FromStream<T>#5651
Merged
kirankumarkolli merged 7 commits intomasterfrom Mar 11, 2026
Merged
Serializer: Fixes unsafe stream cast in FromStream<T>#5651kirankumarkolli merged 7 commits intomasterfrom
kirankumarkolli merged 7 commits intomasterfrom
Conversation
Replaces the unsafe (T)(object)stream cast pattern with safe 'is' pattern matching in all FromStream<T> implementations. The old pattern could throw an InvalidCastException when T was a Stream subclass (e.g., MemoryStream) but the runtime stream was a different Stream type. The fix uses 'stream is T typedStream' for safe runtime type checking and throws a descriptive InvalidCastException when the types are incompatible. Fixed in 7 files (2 core SDK serializers, 3 samples, 2 encryption modules). Added unit tests for both CosmosJsonDotNetSerializer and CosmosSystemTextJsonSerializer confirming the fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kirankumarkolli
requested changes
Mar 4, 2026
Replaces the 3-part block (typeof guard + is check + throw) with a simple 'if (stream is T typedStream)' pattern match across all serializer files. Removes incompatible stream type tests since mismatched cases now fall through to deserialization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
NaluTripician
commented
Mar 4, 2026
Contributor
Author
NaluTripician
left a comment
There was a problem hiding this comment.
Addressed both comments — simplified to just if (stream is T typedStream) { return typedStream; } across all files.
Re: 'Is above if still needed?' — Good call, the outer typeof(Stream).IsAssignableFrom(typeof(T)) guard is no longer needed. The stream is T pattern match already handles the type check at runtime.
Re: 'Isn't it supposed to fallback to the copy?' — You're right. Removed the throw and let incompatible cases fall through to the normal deserialization path. The simplified stream is T check handles both concerns cleanly.
kirankumarkolli
previously approved these changes
Mar 5, 2026
…ypes The simplified 'stream is T' check was too broad - when T=object (e.g. dynamic), it always matched, returning the raw stream instead of deserializing. Restored the typeof(Stream).IsAssignableFrom(typeof(T)) guard in a combined condition. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kirankumarkolli
approved these changes
Mar 11, 2026
This was referenced Mar 17, 2026
4 tasks
5 tasks
This was referenced Mar 23, 2026
NaluTripician
added a commit
that referenced
this pull request
Mar 24, 2026
## Description Fixes #5620 Replaces the unsafe `(T)(object)stream` cast pattern with safe `is` pattern matching in all `FromStream<T>` serializer implementations across the SDK. ### Problem The `FromStream<T>` method in multiple serializer implementations uses the following pattern: ```csharp if (typeof(Stream).IsAssignableFrom(typeof(T))) { return (T)(object)stream; } ``` `typeof(Stream).IsAssignableFrom(typeof(T))` returns `true` when `T` is `Stream` **or any subclass** (e.g., `MemoryStream`, `FileStream`). If `T` is a specific `Stream` subclass but the runtime `stream` parameter is a different `Stream` type, the cast `(T)(object)stream` throws a raw `InvalidCastException` with no context about what went wrong. **Example that throws:** ```csharp // T = MemoryStream, but stream is actually a FileStream at runtime serializer.FromStream<MemoryStream>(someFileStream); // InvalidCastException! ``` ### Fix Replaced the unsafe cast with safe `is` pattern matching: ```csharp if (typeof(Stream).IsAssignableFrom(typeof(T))) { if (stream is T typedStream) { return typedStream; } throw new InvalidCastException( $"Stream of type '{stream.GetType().FullName}' is not compatible " + $"with the requested type '{typeof(T).FullName}'."); } ``` This provides: - ✅ Safe runtime type checking (no unexpected `InvalidCastException`) - ✅ A descriptive error message identifying both the actual and expected types - ✅ No behavioral change for the common case (`T = Stream`) ### Files Changed **Core SDK Serializers (2):** - `Microsoft.Azure.Cosmos/src/Serializer/CosmosJsonDotNetSerializer.cs` - `Microsoft.Azure.Cosmos/src/Serializer/CosmosSystemTextJsonSerializer.cs` **Sample Code (3):** - `Microsoft.Azure.Cosmos.Samples/Usage/SystemTextJson/CosmosSystemTextJsonSerializer.cs` - `Microsoft.Azure.Cosmos.Samples/Usage/ReEncryption/ReEncryptionSupport/ReEncryptionJsonSerializer.cs` - `Microsoft.Azure.Cosmos.Samples/Usage/ItemManagement/Program.cs` **Encryption Modules (2):** - `Microsoft.Azure.Cosmos.Encryption/src/CosmosJsonDotNetSerializer.cs` - `Microsoft.Azure.Cosmos.Encryption.Custom/src/Common/CosmosJsonDotNetSerializer.cs` **Unit Tests (2):** - `Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosJsonSerializerUnitTests.cs` — 2 new tests - `Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Json/CosmosSystemTextJsonSerializerTest.cs` — 2 new tests ### Not Changed - **`PatchOperationCore{T}.cs`** — Has a similar-looking pattern but casts FROM `T` TO `Stream` (upcast), which always succeeds. No fix needed. - **Test utility serializers** — Internal test code, not customer-facing. ### Testing - 4 new unit tests added (2 per serializer): - `ValidateFromStreamWithBaseStreamType` / `TestFromStreamWithBaseStreamType` — Confirms `FromStream<Stream>(memoryStream)` succeeds (regression test) - `ValidateFromStreamWithIncompatibleStreamTypeThrowsDescriptiveError` / `TestFromStreamWithIncompatibleStreamTypeThrowsDescriptiveError` — Confirms `FromStream<FileStream>(memoryStream)` throws `InvalidCastException` with a descriptive message containing both type names - All existing tests continue to pass --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Mar 26, 2026
Bump Microsoft.Azure.Cosmos from 3.57.1 to 3.58.0
azureossd/general-database-connectivity-samples#27
Merged
Merged
This was referenced Apr 6, 2026
This was referenced Apr 13, 2026
This was referenced Apr 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #5620
Replaces the unsafe
(T)(object)streamcast pattern with safeispattern matching in allFromStream<T>serializer implementations across the SDK.Problem
The
FromStream<T>method in multiple serializer implementations uses the following pattern:typeof(Stream).IsAssignableFrom(typeof(T))returnstruewhenTisStreamor any subclass (e.g.,MemoryStream,FileStream). IfTis a specificStreamsubclass but the runtimestreamparameter is a differentStreamtype, the cast(T)(object)streamthrows a rawInvalidCastExceptionwith no context about what went wrong.Example that throws:
Fix
Replaced the unsafe cast with safe
ispattern matching:This provides:
InvalidCastException)T = Stream)Files Changed
Core SDK Serializers (2):
Microsoft.Azure.Cosmos/src/Serializer/CosmosJsonDotNetSerializer.csMicrosoft.Azure.Cosmos/src/Serializer/CosmosSystemTextJsonSerializer.csSample Code (3):
Microsoft.Azure.Cosmos.Samples/Usage/SystemTextJson/CosmosSystemTextJsonSerializer.csMicrosoft.Azure.Cosmos.Samples/Usage/ReEncryption/ReEncryptionSupport/ReEncryptionJsonSerializer.csMicrosoft.Azure.Cosmos.Samples/Usage/ItemManagement/Program.csEncryption Modules (2):
Microsoft.Azure.Cosmos.Encryption/src/CosmosJsonDotNetSerializer.csMicrosoft.Azure.Cosmos.Encryption.Custom/src/Common/CosmosJsonDotNetSerializer.csUnit Tests (2):
Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosJsonSerializerUnitTests.cs— 2 new testsMicrosoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Json/CosmosSystemTextJsonSerializerTest.cs— 2 new testsNot Changed
PatchOperationCore{T}.cs— Has a similar-looking pattern but casts FROMTTOStream(upcast), which always succeeds. No fix needed.Testing
ValidateFromStreamWithBaseStreamType/TestFromStreamWithBaseStreamType— ConfirmsFromStream<Stream>(memoryStream)succeeds (regression test)ValidateFromStreamWithIncompatibleStreamTypeThrowsDescriptiveError/TestFromStreamWithIncompatibleStreamTypeThrowsDescriptiveError— ConfirmsFromStream<FileStream>(memoryStream)throwsInvalidCastExceptionwith a descriptive message containing both type names